Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testapi: Adapt check_screen timeout default to proposal in documentation #965

Merged
merged 1 commit into from Jul 21, 2018

Conversation

okurz
Copy link
Member

@okurz okurz commented May 24, 2018

The doc string of the method check_screen already suggested to use a timeout
of 0 seconds for a long time. This change adjusts the timeout to a default of
0 seconds. This can be considered a breaking change :)

@okurz okurz changed the title testapi: Adapt check_screen timeout default to proposal in documentation [DISCUSSION] testapi: Adapt check_screen timeout default to proposal in documentation May 24, 2018
@okurz
Copy link
Member Author

okurz commented May 24, 2018

@AdamWill WDYT?

@AdamWill
Copy link
Contributor

we have exactly two occurrences of check_screen without an explicit timeout, so I guess whatevs? :D

but, why all the apparently-unrelated changes to doc files and stuff? lint problems?

The doc string of the method check_screen already suggested to use a timeout
of 0 seconds for a long time. This change adjusts the timeout to a default of
0 seconds. This can be considered a breaking change :)
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request May 24, 2018
…nt obvious

This also serves as preparation for
os-autoinst/os-autoinst#965 which changes the default
timeout of check_screen from 30 seconds to 0.
@okurz
Copy link
Member Author

okurz commented May 24, 2018

hmpf, auto-updated doc-file from tests, bad idea :( Excluded the doc-file, thx

@asmorodskyi @coolo ?

@asmorodskyi
Copy link
Member

LGTM

Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I'm not the one who has to fix the test code, I have no objections, too.

@coveralls
Copy link

coveralls commented May 25, 2018

Coverage Status

Coverage decreased (-0.09%) to 57.226% when pulling 002b7e2 on okurz:feature/check_screen into 30f9688 on os-autoinst:master.

@coolo coolo changed the title [DISCUSSION] testapi: Adapt check_screen timeout default to proposal in documentation testapi: Adapt check_screen timeout default to proposal in documentation Jun 4, 2018
@coolo
Copy link
Contributor

coolo commented Jun 4, 2018

@coolo coolo added the not-ready label Jun 4, 2018
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jun 8, 2018
…nt obvious

This also serves as preparation for
os-autoinst/os-autoinst#965 which changes the default
timeout of check_screen from 30 seconds to 0.
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jun 24, 2018
…nt obvious

This also serves as preparation for
os-autoinst/os-autoinst#965 which changes the default
timeout of check_screen from 30 seconds to 0.
@okurz
Copy link
Member Author

okurz commented Jun 27, 2018

test adaptions have been merged, good to go

@okurz
Copy link
Member Author

okurz commented Jul 20, 2018

hello?

@AdamWill
Copy link
Contributor

in case anyone was waiting for me: I updated our tests to be OK with this change ages ago, no objection to merging this.

@foursixnine foursixnine merged commit 3f5ca00 into os-autoinst:master Jul 21, 2018
@okurz okurz deleted the feature/check_screen branch July 21, 2018 19:50
@foursixnine
Copy link
Member

I forgot to remove the notready label :)

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jul 25, 2018
Provide a sane non-zero timeout for the "check_screen" call in
"init_desktop_runner" which is used in "x11_start_program" as after
os-autoinst/os-autoinst#965
the command "check_screen" has a default timeout of 0 seconds and not 30
anymore.

Related progress issue: https://progress.opensuse.org/issues/38837
okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Jul 25, 2018
Provide a sane non-zero timeout for the "check_screen" call in
"init_desktop_runner" which is used in "x11_start_program" as after
os-autoinst/os-autoinst#965
the command "check_screen" has a default timeout of 0 seconds and not 30
anymore.

Related progress issue: https://progress.opensuse.org/issues/38837
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants